Conversation
|
👋 I see @valentinewallace was un-assigned. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4397 +/- ##
==========================================
+ Coverage 86.03% 86.06% +0.03%
==========================================
Files 156 156
Lines 103092 103384 +292
Branches 103092 103384 +292
==========================================
+ Hits 88694 88978 +284
- Misses 11885 11893 +8
Partials 2513 2513
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
TheBlueMatt
left a comment
TheBlueMatt
left a comment
There was a problem hiding this comment.
Because review bandwidth is rather limited, please don't open a flood of PRs at once - give each new PR some time to get review cycles before you open up new ones. If you're looking for ways to contribute, try reviewing existing open PRs! Learning the codebase through review is likely to be way more fruitful than just writing code.
lightning/src/util/persist.rs
Outdated
| .is_err()); | ||
| } | ||
|
|
||
| // Test that a monitor with a legacy u64::MAX update_id (from pre-0.1 LDK) can still be read |
There was a problem hiding this comment.
By "upgrade test", the issue meant adding a test in the lightning-tests crate which actually uses an old version of LDK to get us into a state where we have to handle u64::MAX updates.
There was a problem hiding this comment.
Alright, thank you for your review. Will work on this.
|
I don't see where this uses |
The previous commit used nightly rustfmt which formats differently from the 1.75.0 rustfmt that CI actually uses.
| bitcoin::BlockHash, | ||
| lightning::chain::channelmonitor::ChannelMonitor< | ||
| lightning::util::test_channel_signer::TestChannelSigner, | ||
| >, |
There was a problem hiding this comment.
Instead of storing the monitor and loading it and inspecting it, let's use the async monitor updating flow (setting the return value for the write in the first run to InProgress) and then doing a normal load of the channelmanager after that and checking that it can continue operating normally (and that the pending monitor update is still provided to us and, yes, written through a MonitorUpdatingPersister).
…ngPersister Addresses reviewer feedback on PR lightningdevkit#4397: - Use InProgress persist in the 0.0.125 phase to simulate the async monitor updating flow with a pending u64::MAX monitor update - Reload the ChannelManager from serialized state using a TestChainMonitor backed by MonitorUpdatingPersister - Verify the monitor is persisted through MonitorUpdatingPersister with the sentinel prefix and can be read back correctly
3 participants
legacy_closed_channel_update_id_upgradethat verifies the upgrade path for nodes carrying legacyu64::MAXupdate IDs from pre-0.1 LDK.Closes: #4104